Add support for CNCF model-spec artifact (ModelPacks) #1000
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for the ModelPack format alongside the existing KitOps ModelKit format. The changes refactor media type handling into a separate package, introduce format-aware processing for manifests and layers, and enable the codebase to read, write, and unpack both ModelKit and ModelPack artifacts.
- Refactored media type handling from a simple struct to a more flexible interface-based design in
pkg/lib/constants/mediatype - Added support for reading and unpacking ModelPack artifacts, including generating a minimal Kitfile when one is not available
- Enhanced the pack command to optionally create ModelPack-formatted artifacts via
--use-model-packflag
Reviewed Changes
Copilot reviewed 38 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/lib/constants/mediatype/*.go | New mediatype package with interface-based design supporting both KitOps and ModelPack formats |
| pkg/lib/repo/util/reference.go | Updated to use new mediatype package and handle ModelPack manifests without embedded Kitfiles |
| pkg/lib/filesystem/unpack/core.go | Enhanced to support unpacking ModelPack artifacts and generating minimal Kitfiles |
| pkg/lib/filesystem/local-storage.go | Updated to support creating manifests in either format with appropriate config handling |
| pkg/lib/filesystem/tar.go | Refactored to use new mediatype interfaces |
| pkg/cmd/pack/cmd.go | Added --use-model-pack flag to enable ModelPack format output |
| Various list/inspect commands | Updated to handle artifacts that may not have Kitfiles |
Comments suppressed due to low confidence (2)
pkg/lib/filesystem/tar.go:93
- The error message 'Unsupported compression format' should start with a lowercase letter following Go conventions since it will be wrapped by callers.
pkg/lib/filesystem/unpack/core.go:135 - The error from ParseMediaType is being silently ignored with just a warning log. The code then proceeds to call
mediaType.Base()on line 136, which could cause issues if ParseMediaType returned an error and mediaType is not properly initialized. Consider using a continue statement here to skip this layer when an error occurs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Snyk is reporting two issues with the Currently, our |
amisevsk
changed the title
gorkem
approved these changes
Nov 1, 2025
| return model | ||
| } | ||
|
|
||
| func (kf *KitFile) collectLicenses() []string { |
There was a problem hiding this comment.
Should the licenses be deduplicated?
There was a problem hiding this comment.
Good point; makes sense since licenses aren't tied to individual components in this list.
|
Nice! Could you also add a document in https://github.com/modelpack/model-spec/tree/main/docs to explain how to use kitops with modelpack? |
Move ignore-related code to a subpackage of filesystem and rename interface for simplicity Move layer/modelkit saving code to filesystem package, since it's a better fit there than the kitfile package Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
Add flag --use-model-pack to kit pack command. If specified, instead of packing a ModelKit, kit will pack the Model in the CNCF ModelPack format. Currently, only packing tar-formatted layers is supported; 'raw' layer types are not. Additionally, packing in ModelPack format will not generate a ModelPack config. Signed-off-by: Angel Misevski <[email protected]>
* Handle non-ModelKit manifest types * Find Kitfiles in Kit-generated ModelPacks Signed-off-by: Angel Misevski <[email protected]>
The diff command was using the oras reference parser, which doesn't work for local storage references as it requires a registry and repository. Instead, use our parser, which includes placeholders for registry and repository if not present. Signed-off-by: Angel Misevski <[email protected]>
Fix unpack to handle artifacts where Kitfiles are stored in layers instead of the config section (e.g. ModelPacks). Additionally, (more) gracefully handle unrecognized mediatypes within layers (since we hope other tools would do the same for Kitfiles in layers). Signed-off-by: Angel Misevski <[email protected]>
* Fix "is a Model" check to include ModelPack * Handle descriptors using a 'data' field to support empty descriptors (used in ModelPack config currently) Signed-off-by: Angel Misevski <[email protected]>
Convert Kitfile + model information to a model-spec defined configuration and store it in ModelPack artifacts Includes kind of messy handling for DiffIDs as required by the model-spec format. Signed-off-by: Angel Misevski <[email protected]>
Since we can't trust tools will handle unknown layers in a ModelPack artifact (and also modctl stores its metadata in an annotation) we need to be able to read Kitfiles from an annotation if the artifact is a ModelPack Signed-off-by: Angel Misevski <[email protected]>
If Kit generates a model-spec artifact, it will include the Kitfile with the manifest. However, if the artifact was generated by another tool, we can still try to unpack it based on model-spec metadata annotations (e.g. file names). This is more error-prone, so a warning is logged in this case. Signed-off-by: Angel Misevski <[email protected]>
Since we're adding support for model-spec artifacts (modelpacks), there
may be cases where we are handling artifacts that do not have a Kitfile
available. As a result, we need to handle the case where a config
(Kitfile) cannot be found:
* Add error type for repo utilities to signify that everything is fine,
except no Kitfile is available
* For inspect, warn that there is no Kitfile and show the manifest
* For list, ignore metadata that we normally retrieve from the Kitfile
(e.g. author and model name)
* For unpack, attempt to generate a dummy config to allow unpacking to
continue:
* Don't unpack Kitfile, as it does not exist
* Use model-spec file path annotations to fill a fake Kitfile with
paths to allow unpacking
Signed-off-by: Angel Misevski <[email protected]>
Since we're turning all licenses in a Kitfile into a flat list, it makes sense to deduplicate them, as repeated licenses do not provide any additional information. Signed-off-by: Angel Misevski <[email protected]>
amisevsk
force-pushed
the
modelpack
branch
2 times, most recently
from
92690b2 to
86f1168
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add support for the CNCF model-spec format:
--use-model-packflag tokit pack*+rawmedia types in model-spec, nor does it support zstd compression, though the groundwork is theremodctldoes for its Modelfile format.org.cncf.model.filepathmodel-spec annotation for filenamePart of this PR is a fairly significant refactor of how mediatypes are handled. Mediatypes are now handled through an interface that should make it easy to handle both ModelKits and ModelPack artifacts in a similar way. One fairly significant departure is that Kit can no longer assume every artifact it processes has a Kitfile (as ModelPack artifacts not created by Kit generally won't have one), which introduces a number of additional changes.
Note for testing this PR against
modctl: current releases of modctl use different mediatypes than the model-spec definition, so some commands may not be compatible. For details, see issue modelpack/modctl#317Testing
I've pushed a sample ModelPack artifact as generated by Kit to
jozu.ml/angel/modelpack:kit-generated-modelpack. This is a simple use case, where we're not packing directories (and especially not nested/intersecting directories) and also hasn't been tested againstmodctl(see above issue)For testing, you should be able to do all normal KitOps commands with this artifact (i.e. pull, unpack, pack, inspect, etc). To see the manifest, you can use
kit inspect. To peek at the config in ModelPack format, you can use crane:Linked issues
Closes #872
AI-Assisted Code